Skip to content

feat: add cargo hack check on ci #6262

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 51 commits into
base: develop
Choose a base branch
from
Open

feat: add cargo hack check on ci #6262

wants to merge 51 commits into from
+6,989 −6,725

Conversation

Jiloc
Copy link
Contributor

@Jiloc Jiloc commented Jul 7, 2025 •
edited
Loading

Description

This PR builds on top of #6239, and will be rebased once the other PR will be merged.

The only three changes are:

  • .github/workflows/cargo-hack-check.yml
  • .github/workflows/ci.yml
  • .github/workflows/constants-check.yml

This PR introduces a new, reusable action workflow (cargo-hack-check.yml) to verify that all feature flag in our workspace compile successfully.

The goal is not to accidentally break our support for wasm in future PRs, and prevent accedentally break feature flags like it recently happened in #6228

It is structured into three main stages:

  • setup: A quick preliminary job that reads the Rust toolchain version from the rust-toolchain file. This value is passed to subsequent jobs to ensure they all use the correct and consistent Rust version.
  • Parallel Checks: Two jobs run in parallel:
    • native-targets: This job handles builds for linux and windows (x86_64-unknown-linux-gnu and x86_64-pc-windows-gnu). It runs cargo hack check across all crates and for each feature individually. It would be possible to also do it for all permutations, but it would take too long for a CI work, I suppose.
    • wasm-targets: This job specifically handles our WASM builds. It uses a matrix strategy to run two separate checks for our distinct WASM use cases:
      • One for the wasm-web feature set.
      • One for the wasm-deterministic feature set.
        This job has fail-fast: false so that a failure in one WASM check doesn't prevent the other from running.

Also: .github/workflows/core-build-tests.yml was renamed to ‎.github/workflows/constants-check.yml. This workflow contained a subset of the cargo checks that we are now performing via cargo hack. After removing those check, I thought of renaming it.

Applicable issues

  • fixes #

Additional info (benefits, drawbacks, caveats)

Checklist

  • Test coverage for new or modified code paths
  • Changelog is updated
  • Required documentation changes (e.g., docs/rpc/openapi.yaml and rpc-endpoints.md for v2 endpoints, event-dispatcher.md for new events)
  • New clarity functions have corresponding PR in clarity-benchmarking repo
  • New integration test(s) added to bitcoin-tests.yml

Jiloc added 21 commits June 30, 2025 15:19
@Jiloc
make http-parser and ctrlc optional features
82fbfb1
@Jiloc
remove regex from stacks-common
cb16f1a
@Jiloc
move serde_json to workspace. refactor stacks common cargo
f348b65
@Jiloc
removed time
6775c85
@Jiloc
add vm feature flag
7442358
@Jiloc
fix format
75b0191
@Jiloc
clippy
5e2eeac
@Jiloc
make rand optional for CosmWasm support
d186115
@Jiloc
Merge branch 'chore/stacks-common-optional-requirements' into chore/c…
a9c14bf 
…larity-vm-feature-gate
@Jiloc
substitute ::generate with ::from_bytes
97c968c
@Jiloc
Merge branch 'chore/stacks-common-optional-requirements' into chore/c…
d64ad0b 
…larity-vm-feature-gate
@Jiloc
add custom dummy getdefault
f6621cb
@Jiloc
Merge branch 'chore/stacks-common-optional-requirements' into chore/c…
526c0d1 
…larity-vm-feature-gate
@Jiloc
remove getrandom::register_custom_getrandom for wasm-deterministic
309e98d
@Jiloc
Merge branch 'chore/stacks-common-optional-requirements' into chore/c…
9f9d721 
…larity-vm-feature-gate
@Jiloc
remove unused rand-chacha
5405302
@Jiloc
proxy stacks-common wasm features in clarity
492b0e0
@Jiloc
update Cargo.lock
c007bc6
@Jiloc
add git hack check workflow
0f9fecb
@Jiloc
Merge branch 'develop' into chore/clarity-vm-feature-gate
018885a
@Jiloc
Merge branch 'chore/clarity-vm-feature-gate' into feature/cargo-hack-…
9703978 
…check-ci
@Jiloc Jiloc added this to the 3.1.0.0.14 milestone Jul 7, 2025
@Jiloc Jiloc self-assigned this Jul 7, 2025
@Jiloc Jiloc requested review from a team as code owners July 7, 2025 14:49
@Jiloc Jiloc added the CI label Jul 7, 2025
@Jiloc Jiloc moved this to Status: In Review in Stacks Core Eng Jul 7, 2025
@aldur aldur requested a review from wileyj July 7, 2025 15:14
@Jiloc
Copy link
Contributor Author

Jiloc commented Jul 9, 2025

Overall this looks fine! I’ve added a few minor remaks.

One clarification: the PR description mentions a check-results stage as a final summary job, but I don’t see it defined in the cargo-hack-check.yml workflow. Was this step intended but not included, or is the description out of date?

The initial implementation contained the final check, but I removed it in a later commit 66cf8ba, as I realized it was redundant. But I forgot to remove it from the description 👍

wileyj
wileyj reviewed Jul 9, 2025
View reviewed changes
wileyj
wileyj reviewed Jul 9, 2025
View reviewed changes
@codecov Codecov
Copy link

codecov bot commented Jul 10, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 85.12793% with 279 lines in your changes missing coverage. Please review.

Project coverage is 81.97%. Comparing base (381ee9b) to head (75c4155).
Report is 16 commits behind head on develop.

Files with missing lines Patch % Lines
clarity/src/vm/costs/tracker.rs 82.90% 159 Missing ⚠️
clarity/src/vm/ast/ast_builder.rs 82.43% 75 Missing ⚠️
clarity/src/vm/core.rs 90.41% 37 Missing ⚠️
clarity/src/vm/analysis/engine.rs 94.94% 5 Missing ⚠️
stacks-common/src/util/secp256k1/native.rs 0.00% 2 Missing ⚠️
clarity/src/vm/representations.rs 91.66% 1 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff             @@
##           develop    #6262      +/-   ##
===========================================
- Coverage    82.10%   81.97%   -0.13%     
===========================================
  Files          546      546              
  Lines       347235   347218      -17     
  Branches       323      323              
===========================================
- Hits        285088   284626     -462     
- Misses       62139    62584     +445     
  Partials         8        8
Files with missing lines Coverage Δ
clarity/src/libclarity.rs 55.55% <ø> (ø)
...larity/src/vm/analysis/arithmetic_checker/tests.rs 98.51% <100.00%> (ø)
...c/vm/analysis/type_checker/v2_1/tests/contracts.rs 95.69% <100.00%> (ø)
clarity/src/vm/ast/parser/v2/lexer/mod.rs 94.76% <100.00%> (ø)
clarity/src/vm/ast/parser/v2/parser_impl.rs 55.46% <ø> (ø)
clarity/src/vm/callables.rs 96.50% <ø> (+0.12%) ⬆️
clarity/src/vm/contexts.rs 91.76% <ø> (ø)
clarity/src/vm/costs/mod.rs 42.85% <ø> (-39.86%) ⬇️
clarity/src/vm/database/sqlite.rs 81.97% <ø> (ø)
clarity/src/vm/docs/mod.rs 90.09% <ø> (ø)
... and 19 more

... and 50 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 381ee9b...75c4155. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@wileyj
Copy link
Collaborator

wileyj commented Jul 11, 2025

lgtm - waiting on #6239 to merge so this PR is only for the workflow changes

@wileyj wileyj moved this from Status: In Review to Status: 💻 In Progress in Stacks Core Eng Jul 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wileyj wileyj wileyj left review comments

@fdefelici fdefelici fdefelici left review comments

@fabergat fabergat Awaiting requested review from fabergat

At least 2 approving reviews are required to merge this pull request.

Assignees

@Jiloc Jiloc

Labels
CI clarity-wasm
Projects
Stacks Core Eng
Status: Status: 💻 In Progress
Milestone
3.1.0.0.14
Development

Successfully merging this pull request may close these issues.
3 participants
@Jiloc @wileyj @fdefelici